Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Conversation

@JerrySentry
Copy link
Contributor

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@codecov-notifications
Copy link

codecov-notifications bot commented Dec 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@codecov
Copy link

codecov bot commented Dec 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.01%. Comparing base (e3b3e49) to head (8cc52a3).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1040   +/-   ##
=======================================
  Coverage   96.00%   96.01%           
=======================================
  Files         828      828           
  Lines       19397    19429   +32     
=======================================
+ Hits        18623    18655   +32     
  Misses        774      774           
Flag Coverage Δ
unit 92.29% <100.00%> (+0.01%) ⬆️
unit-latest-uploader 92.29% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link
Contributor

✅ All tests successful. No failed tests were found.

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

@JerrySentry JerrySentry marked this pull request as ready for review December 16, 2024 18:13
@JerrySentry JerrySentry requested review from a team as code owners December 16, 2024 18:13
self._log_updated([owner])

def post(self, request: HttpRequest, *args, **kwargs) -> Response:
def post(self, request: HttpRequest, *args: Any, **kwargs: Any) -> Response:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we know what the args/kwargs are from stripe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code doesn't use args/kwargs at all, I think we just add it in as part of the DRF APIView interface.

return []
commit_sha = self.commit_sha or profiling_commit.commit_sha

if profiling_commit is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there something wrong with the one on the left? Or is this mostly preference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm good point this change is actually not equivalent, old code gives self.commit_sha preference whereas as this gives profiling_commit.commit_sha preference. I need to do it like this:

        commit_sha = None
        if self.commit_sha:
            commit_sha = self.commit_sha
        elif profiling_commit:
            commit_sha = profiling_commit.commit_sha

mypy didn't like the old style because profiling_commit might be null so it can produce an error

Copy link
Contributor

@adrian-codecov adrian-codecov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left comments on the ones I had questions on. Tyvm, this is a great improvement

@JerrySentry JerrySentry added this pull request to the merge queue Dec 16, 2024
Merged via the queue into main with commit e0a8e09 Dec 16, 2024
19 checks passed
@JerrySentry JerrySentry deleted the dec_10_mypy_1 branch December 16, 2024 18:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants